Skip to content

Stateful models: keyed shareable instances, instance subscriptions, and a bank example that demonstrates them (closes #18) - #20

Merged
Yaraslaut merged 42 commits into
masterfrom
feat/stateful-models
Aug 3, 2026
Merged

Stateful models: keyed shareable instances, instance subscriptions, and a bank example that demonstrates them (closes #18)#20
Yaraslaut merged 42 commits into
masterfrom
feat/stateful-models

Conversation

@Yaraslaut

@Yaraslaut Yaraslaut commented Jul 27, 2026

Copy link
Copy Markdown
Member

Closes #18.

What the survey found

Issue #18 asked what morph is missing versus Axelor, Jmix, Causeway and Orleans. Reading those feature lists side by side produces a ~25-item backlog, most of which morph should never build — it owns neither a database, a process engine, nor an IDE. So the survey was done the other way round: against use cases in examples/bank, asking where a developer hits a wall.

That produced one finding that subsumes most of the others:

morph's models were anonymous and stateless, so the model layer was doing nothing. Every bank model's only member was the std::optional<DataMapper> inherited from WithMapper — a connection, not domain state. The per-instance strand morph advertises as its core service therefore protected nothing, and because a BridgeHandler registers one instance, the desktop GUI held five AccountModel instances (and five SQLite connections) for one logical thing.

Orleans names this directly — a grain is identity + behaviour + state, and morph had only behaviour. The fix is not to become an actor runtime; it is to make the existing model layer do what it already claims.

docs/todo.md §F records the full analysis, including a refusal table so the survey does not get re-run: entity metamodels, BPMN, field-level permissions, clustering, reporting engines and the rest, each with the reason it is out of identity.

What shipped

F1 — examples/bank reshaped onto stateful models. AccountModel holds one account in memory, keyed by account id, hydrated on first use and written through on mutation. CustomerModel takes the per-owner half (ListAccounts/OpenAccount), which was never account-scoped — the owner field on those DTOs was the symptom of one model doing two jobs.

F2 — Keyed, shareable model instances. A model declares a nested PrimaryKey alias (detected structurally, like views.md's kind/query); actions declare which field carries it via BRIDGE_KEY_FROM, or that their result establishes it via BRIDGE_KEY_FROM_RESULT. BridgeHandler<M, AllowShared> joins a server-side directory keyed on (typeId, primary), so two handlers — in one process or in two clients over one RemoteServer — reach the same instance.

F3 — Instance subscriptions. subscribe<R>(cb) is keyed on the result/state type and fires whenever an R is produced on the attached instance, by any handler attached to it. A subscriber names what it renders, not what somebody else must call to produce it.

Things worth a reviewer's attention

  • A7 changed. closeConnection now releases one reference per attachment rather than erasing. Without this, one client's disconnect destroys an instance another client is still using. Connection-scope membership became a count for the same reason.
  • Shared instances are ownerless. authorizeInstance's documented ownerPrincipal == ctx.principal policy would otherwise reject every client but the creator, defeating cross-client sharing outright. Gating a shared model is authorize's job or the model's own.
  • A public API was removed. The reactive-draft mechanism (set<&A::field>, reset<A>, the action-keyed subscribe, in-flight coalescing) is gone. FlowSession already owned its own draft tuple and merely mirrored into the handler's, so it now gates on ActionValidator and dispatches directly — its public API, the w-*/app-* schema, and WizardView.qml are unchanged. ActionValidator keeps its A1 server-side role. morph is 0.1.0 and VERSIONING.md reserves exactly this latitude.
  • The bank's sharp edge is shown, not hidden. Transfers and bill payments settle inside a SqlTransaction owned by a different model, so they land behind a cached row's back. bank/db/row_versions.hpp bumps a per-row counter and cached readers re-hydrate on a stale version. morph has no cross-instance transaction and the example must not imply otherwise.
  • Not included: cross-client push. Subscription fan-out is per Bridge. Two handlers in one process — the case the bank GUI has — see each other's work; two separate clients do not. A server-initiated notify frame would need both transports to grow an unsolicited-message path, which is its own item and is recorded as such.

Verification

Configuration Result
clang-release 775 tests, 8091 assertions
clang-debug + net + sqlite + load tests 860 tests
gcc-debug + Qt6 (offscreen QML) 910 tests
examples/bank (SQLite/Lightweight) 21 tests, 145 assertions
Bank desktop GUI, CLI, WASM shadow models build clean
Doxygen WARN_AS_ERROR=FAIL_ON_WARNINGS clean
clang-tidy-diff on changed lines clean
scripts/check_spec_citations.sh clean

New tests pin the new semantics: tests/test_shared_instances.cpp (18 cases,
including result-sourced keys and in-place promotion), two real-socket cases in
tests/net/test_socket_backend.cpp covering the sharing wire path, and
examples/bank/tests/test_stateful_account.cpp (4 cases).
tests/test_subscription.cpp was rewritten for the new subscription meaning.

Two things CI caught that were worth fixing properly

A throughput regression on the hot path. publishResult ran on every
successful result — building a std::type_index and copying into a std::any
before the Completion could resolve — work every existing caller paid for a
feature they aren't using. A relaxed atomic subscription count short-circuits
it; measured over 200k executes, throughput is back to parity with master
(~537k/s vs ~548k/s, within noise).

A pre-existing flaky assertion, not a weakened one. The Valgrind job failed
on test_concurrency_invariants.cpp's succeeded > 0. Valgrind serialises every
thread onto one core, so a switcher looping every 1ms can legitimately cancel all
200 in-flight calls and leave zero successes — master passed that line by luck,
not by invariant. The structural property it was asserting (the
snapshot-and-dispatch path still resolves successfully after repeated switching)
is now checked against the quiesced bridge, which tests the same thing without
racing the switcher. The churn assertions — every call resolves, none is lost —
are unchanged.

Docs follow the repo's convention: docs/planned/ is empty again, the keyed-instance design landed as docs/spec/core/shared_instances.md, and the rest folded into bridge.md, ARCHITECTURE.md, workflows_navigation.md, forms.md and the bank README. README.md's "Status & limitations" also loses three claims that §A/§B had already invalidated.

🤖 Generated with Claude Code

Yaraslaut and others added 10 commits July 28, 2026 00:01
Compares morph against Axelor, Jmix, Causeway and Orleans by use case
against examples/bank rather than by feature-list parity. The finding:
every bank model is stateless (its only member is the inherited
DataMapper), so the per-instance strand protects nothing and a
BridgeHandler registering one instance per handler gives the desktop GUI
five AccountModel instances for one logical thing.

Adds §F to docs/todo.md with three accepted items and a refusal table,
and one spec per item under docs/planned/.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A model declares a nested PrimaryKey alias (detected structurally, like
views.md's kind/query); actions declare which field carries it via
BRIDGE_KEY_FROM, or that their result establishes it via
BRIDGE_KEY_FROM_RESULT. BridgeHandler<M, AllowShared> joins a server-side
directory keyed on (typeId, primary), so two handlers -- in one process or
in two clients over one RemoteServer -- reach the same instance.

- wire: `primary`/`shared` envelope fields plus `attach`, `assign` and
  `instances` kinds. All additive; a `shared:false` register is unchanged.
- RemoteServer: (typeId, primary) directory with a cross-connection attach
  count. Shared instances are recorded ownerless, because
  authorizeInstance's documented ownerPrincipal == ctx.principal policy
  would otherwise reject every client but the creator.
- A7 change: closeConnection now releases one reference per attachment
  rather than erasing, so one client's disconnect cannot destroy an
  instance another client still holds. Scope membership became a count for
  the same reason.
- A keyed action re-points the handler; instances never change identity, so
  a key always maps to one instance and no collision case arises.
- Result-sourced keys promote the instance the create ran on (assignPrimary)
  rather than re-pointing to a fresh one, which would strand the new state.

BridgeHandler<Model> is BridgeHandler<Model, NoSharing> and behaves exactly
as before; all 770 pre-existing tests pass unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SocketBackend and QtWebSocketBackend implement registerModelShared,
attachModel, assignPrimary and listInstances, so cross-client sharing works
over the raw-socket and Qt WebSocket transports and not only through
SimulatedRemoteBackend.

Both keep the same synchronous control-call discipline registerModel
already had, and both degrade to the private path on an empty primary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every bank model was stateless -- its only member was the DataMapper
inherited from WithMapper -- so the per-instance strand morph advertises as
its core service protected nothing, and the example demonstrated the bridge
while making the model layer look like a thin RPC shim.

AccountModel now holds one account in memory, keyed by account id, hydrated
on first use and written through on mutation. CustomerModel takes the
per-owner half (ListAccounts/OpenAccount), which was never account-scoped --
the `owner` field on those DTOs was the symptom of one model doing two jobs.

Cross-model writes (transfer, bill payment, loan disbursement) settle inside
a SqlTransaction owned by another model, so they land behind a cached row's
back. bank/db/row_versions.hpp is the smallest honest fix: writers bump a
counter, cached readers re-hydrate on a stale version. Documented as the
example's sharp edge rather than arranged away.

WASM shadow models and the five GUI controllers move with it; the desktop
GUI, CLI, and all 21 bank tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…anism

subscribe<R>(cb) is now keyed on the result/state type and fires whenever an
R is produced on the instance the handler is attached to -- by this handler,
by another handler sharing the instance, or by another screen entirely. A
subscriber names what it renders, not what somebody else must call to
produce it, so adding an action that also yields an R never breaks an
existing subscriber.

Removes the reactive-draft mechanism it supersedes: set<&A::field>,
reset<A>, the action-keyed subscribe, and the in-flight coalescing whose
subtleties only existed because the draft was remote from its validator.
ActionValidator survives with its A1 server-side role; it loses only its
draft-readiness one.

morph::flows::FlowSession is reworked onto direct dispatch. It already owned
its own _drafts tuple and merely mirrored into the handler's, so it now
gates on ActionValidator itself and executes the completed step. Public
FlowSession API, the w-*/app-* schema, and WizardView.qml are unchanged; all
flows/app tests pass untouched.

Subscriptions are held against the binding, not a fixed instance id, so a
re-pointed handler keeps them -- "tell me about the account I am looking at"
keeps working when the user switches accounts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- spec/core/shared_instances.md: the keyed/shared-instance design, moved out
  of docs/planned and rewritten present-tense now that it ships.
- spec/core/bridge.md: "Subscription semantics" rewritten for result-keyed
  instance subscriptions; SubscriberState, set<>, reset<> and tryFireImpl
  removed along with the mechanism they described.
- ARCHITECTURE.md: "Subscriptions and fielded actions" becomes "Instance
  subscriptions", with the new behaviour table.
- forms/workflows_navigation.md, forms/forms.md: FlowSession dispatches
  directly, so recomputeAll now has three call sites and all are
  authoritative -- the client-side display-only one is gone.
- examples/bank/README.md: documents the stateful, keyed models and the
  cross-model staleness edge.
- README.md: corrects three "Status & limitations" claims that §A/§B had
  already invalidated, and states the in-process scope of subscriptions.

docs/planned/ is empty again, as its own convention requires.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The last call site of the removed reactive-draft API. It now subscribes to
PayeeInfo -- the result type -- and asserts the two properties that replaced
the draft readiness gate: an incomplete payee is rejected by the
dispatch-path validator, and a failed action notifies no subscriber.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
publishResult already skipped an entry whose binding had expired, but only
add/removeSubscription ever erased one. A handler destroyed without an
explicit unsubscribe therefore left its entry behind until some other
handler happened to subscribe -- which in a long-lived app with many
transient handlers is never. Prune while already holding the lock and
walking the list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The default attachModel releases the current instance before acquiring the
new one. If the acquire then failed -- a transport error, a server at
maxLiveModels -- the binding kept pointing at the id it had just given up,
so the next execute dispatched to a released instance and got a confusing
"model not found" instead of the documented "handler not bound".

Unbind first and publish the new id only on success.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- model_key.hpp: drop a redundant `typename`, justify the from_chars pointer
  pair, and wrap the two declaration macros in the same
  NOLINT(cppcoreguidelines-macro-usage) block registry.hpp already uses --
  they must emit a template specialisation at global scope, which no function
  template can do.
- test fixtures: annotate why model/action/result types need external linkage
  (glaze reflection cannot see into an anonymous namespace), mark const
  execute overloads [[nodiscard]], and take subscriber values by reference.
- customer_model.cpp: a real latent bug moved verbatim from the old
  account_model.cpp -- QuerySingle's optional was dereferenced unchecked.
  requireUserId proves the row existed a moment earlier, which is not the
  same as proving this query found it; it now throws NotFound.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Yaraslaut
Yaraslaut force-pushed the feat/stateful-models branch from c60a91c to f93ae51 Compare July 27, 2026 22:52
shared_instances.md is explicit that an instance is shared across clients, so
a reader could reasonably assume a result produced by another client on that
instance reaches this client's subscribers. It does not: there is no
server-initiated frame. Say so where the subscription semantics are defined,
not only in todo.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.75510% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
include/morph/core/remote.hpp 94.94% 10 Missing ⚠️
include/morph/core/bridge.hpp 99.32% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

… race

Two things the Valgrind job surfaced.

publishResult ran on every successful result, building a std::type_index and
copying the result into a std::any before the Completion could resolve --
work paid by every existing caller for a feature they are not using. A
relaxed atomic subscription count now short-circuits it. Measured over 200k
executes: 537k/s before, ~548k/s on master, i.e. back to parity.

The concurrency test's `succeeded > 0` was a coin flip, not an invariant.
Valgrind serialises every thread onto one core, so a switcher looping every
1ms can legitimately cancel all 200 in-flight calls and leave zero
successes; master passed it by luck. The structural claim it was making --
that the snapshot-and-dispatch path still resolves successfully after
repeated switching -- is now asserted against the quiesced bridge, which
tests the same property without racing the switcher. The churn assertions
(every call resolves, none is lost) are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Yaraslaut
Yaraslaut force-pushed the feat/stateful-models branch from d44c364 to 38b6e2d Compare July 27, 2026 23:06
Yaraslaut and others added 15 commits July 28, 2026 01:10
BRIDGE_KEY_FROM_RESULT and the assignPrimary promotion it drives had no test
at all -- a real gap, not just a coverage number. The two new cases pin the
property that motivated promoting an instance in place rather than
re-pointing to a fresh one: the state the creating action just built is
still there afterwards, and the instance is then reachable by its generated
key like any other. Covered locally and over SimulatedRemoteBackend, so the
`assign` wire verb is exercised too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SocketBackend's registerModelShared/attachModel/listInstances shipped
exercised only through SimulatedRemoteBackend, which never touches them --
the wire path for sharing had no coverage at all.

Two cases over an actual WebSocket, using a stateful counter because a
stateless echo model cannot tell sharing from not-sharing: two separate
clients naming one key observe a single counter (15, not 5) and one
directory entry, while a plain handler on a third connection registers its
own instance and counts from zero.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Unreleased section had no Changed or Removed headings because nothing had
needed them yet. This program needs both: A7's closeConnection semantics
changed, and the reactive-draft mechanism is a public API removal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…count

The wire-protocol section listed three additive changes and omitted `assign`
entirely -- the verb that makes a result-sourced key promote in place instead
of stranding whatever the creating action just did.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three branches the happy-path tests never reached, all driven through the
public API rather than a test-only accessor:

- a creating action whose generated key collides with a live instance must
  leave the incumbent's state and directory entry alone (ShiCreateAs lets a
  test choose the generated id, which is the only way to force this);
- re-attaching to the key a handler already holds must not release and
  re-acquire the instance, which would silently discard its state;
- instances() before anything is attached is empty, not an error.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Patch coverage showed the new wire surface's server paths were reachable
only through the happy path: 47 of the uncovered lines were in remote.hpp.
These drive RemoteServer directly, which is the only way to reach the
refcount-and-scope interactions that make cross-client sharing safe.

Covers the A7 change concretely -- closing one of two connections sharing a
key leaves the instance alive and executable for the other, and only the
last release destroys it -- plus attach re-pointing, assign promotion, assign
declining to displace an incumbent, the instances listing (with a private
register correctly absent), empty-typeId rejection for all three new kinds,
and a shared register against an already-closed scope.

The directory keys on envelope strings, so no keyed model type is needed to
exercise it -- that is a client-side concept.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A keyed model had to carry `using PrimaryKey = ...` in its own class body and
then repeat the key's type knowledge in a separate BRIDGE_KEY_FROM line. That
is the same fact in two places, free to drift, and it put framework vocabulary
inside a class whose whole selling point is being plain C++.

BRIDGE_MODEL_KEY(Model, Action, &Action::field) now does both jobs from the
one line the author is already writing registrations on: it deduces the key
type from the member pointer (specialising ModelKeyTraits<Model>) and records
Action as the one that carries it. The model class is untouched -- a keyed
model is now indistinguishable from an unkeyed one by inspection.

Split from BRIDGE_KEY_FROM deliberately, because a model usually has several
actions naming the same entity but can only have one explicit
ModelKeyTraits specialisation: BRIDGE_MODEL_KEY appears once per model,
BRIDGE_KEY_FROM marks every further action that carries the key. Same split
for the result-sourced pair.

`using PrimaryKey` still works and still wins -- infer by default, declare to
override -- for the case where the key type differs from the field's.

The bank's AccountModel and CustomerModel drop their aliases accordingly.

Also removes docs/superpowers/2026-07-06-reactive-forms-bridge.md, which
should not have been in the codebase.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Writing the coverage test for the shared path found a real bug. The register
branch's cheap early cap check ran before the shared fast path, so with
maxLiveModels set, a shared register that would merely take another reference
to a live instance was refused with "too many models".

That is exactly backwards: the cap bounds live models, not attachments to
them, and refusing here means a loaded server turns away the second client of
an instance it is already hosting -- precisely when sharing is worth the most.
The authoritative re-test inside acquireSharedInstance already distinguishes
the two, because it runs where the insert does.

Also covers the paths this exposed: IBackend's sharing defaults degrading to
private instances, assignPrimary re-filing and its no-op inputs, a shared
handler surviving switchBackend (local and local->remote), attach/instances
under a denying authorizer, a result key on an already-attached handler, a
subscriber with no executor, and instances() surfacing a key the client
cannot decode rather than silently yielding 0.

Patch coverage over instrumented lines: 92.45% -> 94.68%.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d does

Doxygen builds ARCHITECTURE.md as the mainpage and cannot resolve a markdown
link to docs/spec/, which is outside its input set -- WARN_AS_ERROR turned
that into a docs-build failure. Every other spec reference in this file is
inline code for exactly that reason; this one now matches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Patch coverage over instrumented lines: 94.68% -> 97.43%.

- a change-aware model registered through the shared path is still recorded
  as change-aware, so it keeps being notified across a backend switch;
- the server re-files an instance onto a new key over the wire, dropping the
  old directory entry rather than leaving it reachable under two keys;
- attach/instances stamp an authenticating authorizer's verified principal;
- a refusing server surfaces as an exception on each of the remote backend's
  three control calls, rather than a bogus id or an empty list a caller would
  read as success;
- an empty primary on the remote backend degrades to a private instance.

The change-aware test asserts directly rather than polling:
notifyBackendChanged posts onto the instance's strand and the following
execute posts onto the same strand, so it is ordered strictly after. A poll
there would have hidden a real ordering bug behind a retry -- and an earlier
draft of it did exactly that, calling settle() (which contains REQUIREs)
inside the predicate.

What remains uncovered is a double-checked directory re-test that is by
nature race-only, plus QtWebSocketBackend, which the coverage job does not
build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the single lifetime test with two, addressing review feedback that
the original provided no protection outside a sanitizer build:

- A new deterministic test covers the onResult half of the bug (previously
  uncovered): pre-fix, onResult ran completely unconditionally, so a flag
  set as its first statement -- before it touches the dangling bridge via
  a captured raw pointer, mirroring BridgeHandler::execute's ResultKeyed
  branch -- differs pre/post fix regardless of memory contents. Verified
  empirically: fails 10/10 runs against the reverted pre-fix ordering,
  passes 5/5 against the fix.

- The hasSubscribers() half is now an explicitly-labeled best-effort probe
  (heap-allocated bridge, freed memory scribbled to raise reuse odds)
  rather than presented as a regression guard: verified empirically that
  it does not reliably fail pre-fix (6/6 clean passes against the reverted
  ordering), because the final branch outcome is identical pre/post fix in
  a sequential, single-threaded destroy-then-resolve test -- only a
  sanitizer or a genuine concurrent race can observe the difference. Also
  confirmed this machine's AppleClang ASan+UBSan combination hangs even on
  --list-tests (no test execution at all), independent of anything this
  task touched, so that route is unavailable here.
…age death test

Replace the heap-reuse best-effort probe with one that places the Bridge on
an mmap'd page, destroys it in place, then mprotect's the page to PROT_NONE.
Pre-fix, hasSubscribers() dereferences the protected page and faults
deterministically instead of possibly reading stale-but-intact memory. The
fault is recovered in-process via a sigsetjmp/siglongjmp handler (installed
for both SIGSEGV and SIGBUS -- this machine's Darwin kernel delivers SIGBUS
for a PROT_NONE violation, which Catch2's own signal-handler list does not
include) and converted into a normal, explicit FAIL(), so a regression here
reports as one attributable failed test case rather than killing the whole
process. POSIX-only; compiled out on Windows.

Verified empirically: fails 10/10 runs (clean "FAILED: hasSubscribers()
touched the destroyed bridge..." via caught signal 10) against the reverted
pre-fix ordering, passes 5/5 individually and together with the rest of
[bridge][lifetime] against the fix. Full suite (801 cases, 8234 assertions)
passes in one process post-fix.
Yaraslaut and others added 15 commits August 3, 2026 13:02
makeAttach had no contextKey parameter, so an instance created via its
first attach (rather than a shared register) never reached a configured
LogProvider -- the entity's stable identity was silently dropped on the
wire. Thread identity.contextKey through the three attachModel call sites
(SimulatedRemoteBackend, SocketBackend, QtWebSocketBackend).
…d instance

LocalBackend::assignPrimary and RemoteServer::applyAssignLocked promote only
a still-anonymous instance now: one already filed under a different real key
is left exactly where it is instead of being silently re-filed, which would
otherwise strand any other client still attached under the old key. This
makes the code actually enforce the spec's "instances never change key"
invariant.

Also gate Bridge::assignHandlerPrimary's local bookkeeping the same way: it
must not cache a primary the backend refused to file the instance under,
which is exactly what the rewritten
"a result-sourced key is not promoted when the handler already holds a real
key" test caught.
…nt instance id

ShiHydrateModel carries no observable state, so the existing value-based
assertion cannot distinguish a fresh instance from the reused, poisoned
one. Compare BridgeHandler::binding()->currentId directly, the same way
the remote test already compares modelId.
releaseCurrent used to be released in a standalone step between the two
locked sections of acquireSharedInstance's miss path -- before the
maxLiveModels admission check and before the second lock's re-check hit
branch (which released nothing at all). A subsequently failing admission
check or a throwing construction could therefore strand the caller's
handler: its old instance already gone, its new one never created.

Move the release into the same locked section as the maxLiveModels check
(so a sole holder's release frees exactly the slot the re-point needs
instead of losing it to the cap) and add the missing release on the
re-check hit branch. Update attachHandler's comment in bridge.hpp to match
the now-accurate guarantee: a throwing acquire never touches `previous`
except when a connection scope closes concurrently, which is harmless
since no further request on it will ever run.

Add a maxLiveModels re-point regression test, plus a second test that
directly discriminates the fix via a throwing model construction (the
first test alone happens to pass under the old ordering too, since the
single-threaded case it covers works out arithmetically either way).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…wing

The strand-posted lambda in dispatchExecute reconstructed a local `mid`
from env.modelId, shadowing the outer `mid` used by `_strand.post(mid,
...)`. Both hold the identical value, so this was harmless in practice,
but GCC's -Wshadow (enabled with -Werror in this project) may flag it
even though Clang's captured-variable shadowing rules are more lenient --
a preventive rename to targetMid, scoped to the lambda body only.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… guard

The doc comment implied a guard against promoting onto a target key
already held by another instance. The only guard that actually exists is
against re-keying a binding that already holds a different real primary
(`!binding->primary.empty()`) -- the backend's own assignPrimary silently
declines the already-taken-target-key case with no way for this method to
observe it. Reworded to describe only the real guard and recorded the
residual gap (the binding's cached primary can desync from what the
backend actually filed it under) as a tracked follow-up. No code change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Failure-modes bullet about attaching to a key whose entity does not
exist said a failed first action "releases" the instance -- it doesn't.
Verified against LocalBackend's HydrationFlags/_hydrationFlags and
RemoteServer's _firstActionPending/_poisoned: the instance is marked and
evicted from the directory lazily, on the next attach to that key, not
destroyed immediately; it stays alive (still counting against
maxLiveModels) until whoever created it releases it normally. The handler
that hit the failure keeps its broken instance, since attachHandler's
same-primary no-op guard means retrying the same keyed action never
re-runs the backend attach.

Also add a Limitations bullet: poisoning is checked only against
already-settled first-action failures, not retroactively, so two attaches
racing the same not-yet-existing key can both land on the same instance
while its first action is still in flight.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tables

Task 4 already updated the "contextKey -- stable identity" prose to state
contextKey is carried on both register and attach, but the discriminator
summary table and the Envelope field-reference table still said
"register" only, contradicting it. Updated both rows to mention attach
alongside register. The discriminator table's pre-existing lack of any
row for attach/assign/instances is a larger, separate gap (covered by
shared_instances.md) and is intentionally left untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-page test

Assigning to sa.sa_handler expands glibc's <bits/sigaction.h> macro
(sa_handler -> __sigaction_handler.sa_handler), which clang's
-Wdisabled-macro-expansion flags as a false positive on Linux under
-Weverything -Werror; the BSD-derived <signal.h> this test was
authored and verified against locally doesn't hit it. Scope the
suppression to just this one assignment.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Yaraslaut
Yaraslaut merged commit 7a0d68a into master Aug 3, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Other similar solutions

1 participant